Skip to content

Introduced new test_read_netcdf.py function for directly converting netCDF files into XarrayContext.#226

Open
dfang843 wants to merge 1 commit into
mainfrom
test_read_netcdf
Open

Introduced new test_read_netcdf.py function for directly converting netCDF files into XarrayContext.#226
dfang843 wants to merge 1 commit into
mainfrom
test_read_netcdf

Conversation

@dfang843

Copy link
Copy Markdown
Collaborator

No description provided.

directly converting netCDF files into
XarrayContext.

@alxmrs alxmrs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First (quick) pass. Thanks for the PR!

Comment thread xarray_sql/sql.py
self.register_udf(cft.make_cftime_udf(units, cal))
break # One UDF per context is enough.

@classmethod

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be an instance method.

Comment thread xarray_sql/sql.py
"""
path = Path(path)

ds = xr.open_dataset(path, engine=engine, **open_kwargs)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The engine should be best-effort a netcdf engine IMO, and users won't need to pass it in, since the method name implies the type of data.

Comment thread xarray_sql/sql.py
XarrayContext
A context with the dataset already registered.
"""
path = Path(path)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think open dataset handles this.

Comment thread xarray_sql/sql.py
engine: str | None = "netcdf4",
**open_kwargs,
):
"""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use Google style docstrings.

Comment thread xarray_sql/sql.py
table_name = path.stem

if chunks is None:
chunks = {"time": 24} if "time" in ds.dims else {d: -1 for d in ds.dims}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's be correct, not polite. The users must pass in chunks.

Comment thread xarray_sql/sql.py
from .reader import read_xarray_table

from pathlib import Path
from matplotlib.dates import date2num

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we import this?

Comment thread test_read_netcdf.py
from metpy.cbook import get_test_data
from xarray_sql import XarrayContext

path = get_test_data('irma_gfs_example.nc', False)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use pytest conventions for tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants